-
-
Notifications
You must be signed in to change notification settings - Fork 150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds schema creating csv schema with View #195
Conversation
@Qnzvna this looks very useful, thank you! I think I have some comments wrt implementation, but in general I think I like the idea. Just one practical thing before I can merge this: unless I have asked for CLA: https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf I would one before first contributions (but only then; future contributions can all be under existing CLA then). |
@@ -460,6 +487,18 @@ protected void _addSchemaProperties(SerializerProvider ctxt, CsvSchema.Builder b | |||
BeanDescription beanDesc = ctxt.introspectBeanDescription(pojoType); | |||
final AnnotationIntrospector intr = ctxt.getAnnotationIntrospector(); | |||
for (BeanPropertyDefinition prop : beanDesc.findProperties()) { | |||
if (view != null) { | |||
boolean viewVisible = false; | |||
for(Class<?> propView : prop.findViews()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs null check, findViews()
can return null
. Also note that in databind BeanDeserializerFactory
method addBeanProps()
, there is:
Class<?>[] views = propDef.findViews();
if (views == null) {
views = beanDesc.findDefaultViews();
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, updated.
if (view != null) { | ||
boolean viewVisible = false; | ||
for(Class<?> propView : prop.findViews()) { | ||
if (propView.equals(view)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sufficient: Views actually check inheritance too (so, say, CharSequence
as inclusion would be acceptable for View String
).
You might be able to use databind's ViewMatcher
for checks:
public static ViewMatcher construct(Class<?>[] views) { }
and then calling method:
public boolean isVisibleForView(Class<?> activeView) { ...}
seems like the proper (and still simple) way to go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, updated also.
298bca6
to
07c084c
Compare
I've signed the CLA before when providing different contributions. |
@Qnzvna Ahh! I did find the CLA from 2017. Thank you! |
Excellent: I backport this in 2.11 branch and this will just make it in 2.11.0 thanks to timing. Thank you again for contributing this new feature! |
Thanks for merging so quickly. |
@Qnzvna and 2.11.0 is actually out now! Perfect timing. :) |
No description provided.